Skip to content

Conversation

@averissimo
Copy link
Contributor

@averissimo averissimo commented Jun 12, 2025

New feature for teal.code that keeps the last value of the code evaluation.

In line with R's .Last.value (see doc)

pkgload::load_all("../teal.code")
#> ℹ Loading teal.code

q <- qenv() |> within({
  1+1
}) |> within({
  aa <- 2
})
q
#> <environment: 0x616d80da6ec0> 🔒 
#> Parent: <environment: devtools_shims> 
#> Bindings:
#> - aa: [numeric]

q@.xData |> attributes()
#> $.Last.value
#> [1] 2

library(dplyr) |> suppressPackageStartupMessages()

q2 <- q |> within(
  bb <- 3
)
q2
#> <environment: 0x616d80725698> 🔒 
#> Parent: <environment: package:dplyr> 
#> Bindings:
#> - aa: [numeric]
#> - bb: [numeric]
q2@.xData |> attributes()
#> $.Last.value
#> [1] 3

Cloning

rlang::env_clone(as.environment(q2)) |> attributes()
#> NULL

Created on 2025-06-12 with reprex v2.1.1

image

@averissimo averissimo requested a review from gogonzo June 12, 2025 10:18
@averissimo
Copy link
Contributor Author

Should we add a getter function?

{
eval(current_call, envir = object@.xData)
.Last.value <- eval(current_call, envir = object@.xData)
attr(object@.xData, ".Last.value") <- .Last.value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing to base - .Last.value isn't an attribute but an object of the .GlobalEnv

Copy link
Contributor

@gogonzo gogonzo Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, even worse. .Last.value is an object in base and not assigned to any environment :/

a <- "a"
c <- "c"
a
base::.Last.value
# [1] "a"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we cannot fully reproduce it, it would just follow the concept.

Base automatically changed from teal_reportable to main June 12, 2025 14:44
…table

* origin/main:
  [skip actions] Bump version to 0.6.1.9003
  `{teal}` module returns a `teal_report` object that extends from `teal_data` (#255)
@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2025

Unit Tests Summary

  1 files   13 suites   4s ⏱️
159 tests 156 ✅ 3 💤 0 ❌
249 runs  246 ✅ 3 💤 0 ❌

Results for commit d6c9c0d.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2025

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
qenv_eval_code 👶 $+0.02$ eval_code_keeps_.Last.value_as_an_attribute_of_the_environment
qenv_within 👶 $+0.02$ within_keeps_.Last.value_as_an_attribute_of_the_environment

Results for commit bef1907

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  ---------
R/qenv-c.R                          55       0  100.00%
R/qenv-class.R                      12       0  100.00%
R/qenv-concat.R                      7       0  100.00%
R/qenv-constructor.R                 1       0  100.00%
R/qenv-errors.R                      4       4  0.00%    6-9
R/qenv-eval_code.R                  62       1  98.39%   42
R/qenv-extract.R                    30       0  100.00%
R/qenv-get_code.R                   24       0  100.00%
R/qenv-get_env.R                     3       1  66.67%   27
R/qenv-get_messages.r                5       0  100.00%
R/qenv-get_var.R                    26       0  100.00%
R/qenv-get_warnings.R                5       0  100.00%
R/qenv-join.R                        7       7  0.00%    137-151
R/qenv-length.R                      2       1  50.00%   2
R/qenv-show.R                       29      29  0.00%    19-50
R/qenv-within.R                      8       0  100.00%
R/utils-get_code_dependency.R      245       2  99.18%   160, 258
R/utils.R                           30       0  100.00%
TOTAL                              555      45  91.89%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: d6c9c0d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Attribute of the environment" is not exactly what it is

)
})

testthat::test_that("eval_code keeps .Last.value as an attribute of the environment", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testthat::test_that("eval_code keeps .Last.value as an attribute of the environment", {
testthat::test_that("eval_code stores .Last.value in the parent environment", {

testthat::expect_identical(get_code(q), "a <- 1L")
})

testthat::test_that("within keeps .Last.value as an attribute of the environment", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testthat::test_that("within keeps .Last.value as an attribute of the environment", {
testthat::test_that("within eval_code stores .Last.value in the parent environment", {

testthat::test_that("within keeps .Last.value as an attribute of the environment", {
q <- within(qenv(), x <- 1)
env <- parent.env(q)
testthat::expect_true(".Last.value" %in% names(env))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not needed as the next one confirms this also

Suggested change
testthat::expect_true(".Last.value" %in% names(env))

testthat::test_that("eval_code keeps .Last.value as an attribute of the environment", {
q <- eval_code(qenv(), quote(x <- 1))
env <- parent.env(q)
testthat::expect_true(".Last.value" %in% names(env))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not needed as the next one confirms this also

Suggested change
testthat::expect_true(".Last.value" %in% names(env))


### Enhancements

* Code evaluation keeps the last evaluated expression in the `.Last.value` attribute of the environment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Code evaluation keeps the last evaluated expression in the `.Last.value` attribute of the environment.
* Code evaluation keeps the last evaluated expression in the `parent.env(<qenv>)$.Last.value`.

Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the best proposition so far. I don't think we will be able or it will be worth it to follow the rmarkdown concept of fetching everything like it was called line by line in the interactive session:

image

I think it is a good compromise easy to cache always the .Last.value in qenv. We will work on the teal.reporter followup implementation of this 👍

@averissimo
Copy link
Contributor Author

@gogonzo Should we pause/discard this PR given your new discovery late yesterday and our call today in favor of using evaluate::evaluate() with custom handler.

This could live as its own feature though.

@averissimo
Copy link
Contributor Author

Closed in favor of #259

@averissimo averissimo closed this Jun 25, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2025
@averissimo averissimo deleted the last_value@teal_reportable branch June 25, 2025 15:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants